Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@internal/skills/skills.go`:
- Around line 184-185: The errs variable declared at line 184 is a slice of
strings ([]string), and at lines 205, 210, and 220, errors are being converted
to strings before appending, which destroys the error chain and breaks
errors.Is/As functionality downstream. Change the errs variable from a slice of
strings to a slice of error types ([]error), preserve the actual error objects
when appending to this slice instead of converting them to strings, ensure
individual errors are wrapped with appropriate context using fmt.Errorf with %w
wrapping, and ensure the final aggregated error properly maintains the error
chain so downstream consumers can use errors.Is and errors.As correctly.

In `@pkg/kit/compaction.go`:
- Around line 119-124: The error handling block in the compactInternal function
does not wrap the error returned from m.compactImpl with context information.
Wrap the error using fmt.Errorf with an appropriate contextual message before
using it in both the CompactionEvent emission and the return statement. Use the
%w verb to preserve the original error while adding call-site context that will
aid debugging for subscribers and callers.

In `@pkg/kit/errors.go`:
- Around line 33-40: The documentation for the ClassifyProviderError function
claims "The original error remains reachable via errors.Unwrap", but this is
incorrect because the standard errors.Unwrap function only works with Unwrap()
error (returning a single error), not Unwrap() []error (returning multiple
errors). Verify the actual implementation of the error type returned by
ClassifyProviderError and ensure it properly implements the Unwrap method
according to the standard library contract. If the function returns an error
that implements Unwrap() []error, either change it to implement Unwrap() error
returning a single unwrapped error, or update the documentation to accurately
reflect what can be retrieved via errors.Unwrap.

In `@pkg/kit/kit.go`:
- Around line 2803-2808: The restore closure in the defer block calls
m.SetModel(ctx, prevModel) using the caller's context, which may be canceled or
expired by the time the defer executes, causing the error to be silently ignored
with an underscore. Replace the caller's context with a background context or a
new context that won't be canceled when calling m.SetModel in the restore
function, ensuring the rollback can complete successfully and prevent per-call
overrides from leaking into subsequent calls.
- Around line 2796-2800: Replace the direct m.v.Set() calls for the three
configuration keys (thinking-level, provider-url, and provider-api-key) with the
restoreViperString helper function to properly respect the prev*Set flags that
were saved at the start of the block. Instead of calling
m.v.Set("thinking-level", prevThinking), m.v.Set("provider-url", prevURL), and
m.v.Set("provider-api-key", prevKey) directly, use restoreViperString for each
key with its corresponding previous value and flag, matching the pattern already
used at lines 2803-2806 for the restore() operation.

In `@pkg/kit/sessions.go`:
- Around line 220-227: The CollapseBranch method in the Kit type accepts a toID
parameter that is not used in the implementation, since AppendBranchSummary
always collapses to the current leaf. Add godoc documentation to the
CollapseBranch function that clarifies the toID parameter is unused and
explicitly documents that the method always collapses branches to the current
leaf regardless of what toID value is passed, to prevent callers from being
confused about the actual behavior.

In `@pkg/kit/tools.go`:
- Around line 5-8: The null check comparing `string(input) != "null"` is brittle
because it doesn't handle whitespace variants like " null ", which can cause
json.Unmarshal to unexpectedly set args to nil. Update the null check logic by
using strings.TrimSpace to remove leading and trailing whitespace from the input
before comparing it to "null". Apply this fix to all occurrences of the brittle
null comparison, including the locations around the import section and the
also-applies-to range mentioned in the comment.

In `@www/pages/sdk/sessions.md`:
- Line 102: The text in the SessionManager documentation section contains an
incorrect reference labeled "SDK skill reference" that links to a repository and
appears outdated for this context. Replace the misleading "SDK skill reference"
link text and URL with an appropriate reference that correctly documents the
SessionManager interface definition, ensuring it accurately reflects the actual
documentation or interface definition location being referenced.

---

Outside diff comments:
In `@pkg/kit/kit.go`:
- Around line 2359-2361: The OnReasoningStart callback function currently only
emits ReasoningStartEvent but needs to also emit StreamEventReasoningStart to
ensure TurnResult.Stream has the proper start marker for providers using
reasoning callbacks. In addition to the existing m.events.emit call with
ReasoningStartEvent, add another m.events.emit call to emit
StreamEventReasoningStart with the same ID parameter to properly mark the start
of the stream event on the non-think path.

---

Nitpick comments:
In `@pkg/kit/errors_test.go`:
- Around line 37-41: The unclassified error assertion in the test is comparing
error text representations using the Error() method, which can mask wrapping or
modifications to the error object. Replace the comparison of Error() string
values with a direct pointer identity check comparing got and tc.in directly, so
the test verifies that the exact same error object is returned unchanged rather
than just checking that the text representation matches.

In `@pkg/kit/kit.go`:
- Around line 2801-2802: Error returns at lines 2801-2802 and 2843-2845 lack
operation context for debugging. Wrap each bare error return statement (where
`return nil, err` or similar appears) with `fmt.Errorf` to add descriptive
context about what operation failed, using the pattern `fmt.Errorf("operation
description: %w", err)`. This applies to all error return statements in the
affected code sections to maintain consistency with project coding guidelines.

In `@pkg/kit/rawtool_test.go`:
- Around line 58-72: Add a regression test case within TestNewRawToolInvalidArgs
or create a new test function to verify that NewRawTool handles
whitespace-padded null input (such as " null " with surrounding spaces)
consistently with plain null input. The test should call tool.Run with this
whitespace-null input and verify that the handler is not invoked and the
response indicates proper error handling, ensuring nil-map parsing behavior
remains consistent across different null input variations.